Support skipping integration tests for unavailable CF features via environment variables#1346
Support skipping integration tests for unavailable CF features via environment variables#1346jorbaum wants to merge 14 commits intocloudfoundry:5.x.xfrom
Conversation
0c5a424 to
5ad1072
Compare
There was a problem hiding this comment.
- Really clear annotations for execution conditions, I like the approach.
- A few questions and remarks throughout the PR.
- Please target
5.x.x
My biggest issue is that half of the classes in the v3 package are annotated with @RequiresV2Api. It seems v2 is required because we push applications (service broker, most notably) in the config with the v2 API, and those tests needs application pushed.
Could you please consider updating the core integration test config to use v3 apis for pushing apps instead?
To have examples of this, look at the ApplicationsTest in the v3 package.
|
One big reason for why I wanted to create a draf PR initially (which, apparently I did not :P ) is that I only tested the new annotations in the following combination: SKIP_V2_TESTS=false \
SKIP_TCP_ROUTING_TESTS=true \
SKIP_BROWSER_AUTH_TESTS=true \
SKIP_METRIC_REGISTRAR_TESTS=trueand SKIP_V2_TESTS=true \
SKIP_TCP_ROUTING_TESTS=true \
SKIP_BROWSER_AUTH_TESTS=true \
SKIP_METRIC_REGISTRAR_TESTS=trueThis is because my test environment does not support tcp routing, browser-based auth or cf marketplace. Do you think this is fine? |
|
I think it's fine. I'll test locally before merging, with and without the flags. please also target 5.x.x, we'll merge this back into main later. |
TODO: I am unsure about this one. Needs more exploration # Conflicts: # README.md # integration-test/src/test/java/org/cloudfoundry/operations/StacksTest.java
Some UAA do not return a Location header.
Prevent the serviceBrokerId bean from being created when SKIP_V2_TESTS=true, rather than relying on @Autowired(required=false) in each test class. This avoids the bean's initMethod="block" eagerly calling the V2 API at context startup even when all V2 tests are disabled.
Extract the "SKIP_V2_TESTS" string literal into a public constant RequiresV2Api.SKIP_V2_TESTS_ENV and reference it from IntegrationTestConfiguration and CloudFoundryCleaner.
The info() test alone doesn't bring enough value to justify keeping the class partially enabled without V2.
Replace V2 applicationsV2().listServiceBindings / removeServiceBinding calls with V3 serviceBindingsV3().list / delete in the cleaner's application cleanup path, eliminating the runV2Calls boolean parameter.
Reword to explain the env var is for when UAA doesn't have browser-based auth configured, rather than referencing "non-interactive environments".
Move the SKIP_V2_TESTS check into a reusable static method in V2ApiCondition.
5ad1072 to
25cb827
Compare
I have now addressed all of the comments except for this one. Moving the test config to use v3 seems like a bigger endeavor. I did not try to tackle it so far. Also I did not manage to rerun the integration tests again. Feel free to take a look at my changes already though. |
When SKIP_V2_TESTS=true, the organizationId and spaceId beans were not created (guarded by @ConditionalOnProperty), but V3 test classes like ApplicationsTest autowire them without @RequiresV2Api, causing failures. Replace V2 API calls with V3 equivalents so these beans are always available: - organizationId: use organizationsV3().create() and rolesV3().create() instead of V2 quota definitions, org creation, and manager association - spaceId: use spacesV3().create() with SpaceRelationships instead of V2 space creation Remove the now-unused organizationQuotaName bean.
Add missing state and stateReason fields to the V3 Stack model to match the current CF API response, then migrate the stackId and stackName beans to use stacksV3() instead of the V2 stacks API. This allows StacksTest to run when SKIP_V2_TESTS=true.
|
I now ran this against https://github.com/Lokowandtg/kind-deployment/tree/cf-java-client.integration-test and found a bunch of issues that I fixed with the latest commits. In those I migrated organizationI, spaceId, stackId and stackName from V2 to V3. But refrained from migrating app pushing so far as its a larger endeavor. Also note that the default quota is now used for the organization. I also found a Json deserialization issue in Stack.java that I fixed. I ran the following integration tests:
|
On my CF test environment quite a few integration tests needed features that my environment did not support.
I added a bunch of environment variables so those tests can be disabled at a global level.
I am not sure about this approach yet. Especially regarding disabling V2 CAPI as it touches quite a lot of places. Any feedback welcome!
AI tools used: Claude Code and GitHub Copilot (Opus 4.6) assisted me during development. I reviewed the result.